Add unique_id field to product #47364

nathanss · 2024-05-10T13:25:51Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Adds a new "unique id" field to the new product editor. The field accepts any type of input, at least for now.

Closes #41816
Closes #39577

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

If you're running locally, please deactivate and activate the WooCommerce plugin, as this adds a new column in the wp_wc_product_meta_lookup table.
Enable the new product editor at /wp-admin/admin.php?page=wc-settings&tab=advanced&section=features
Go to Products > Add
Go to the 'Inventory' tab
Check the field labeled "GTIN, UPC, EAN OR ISBN"
You should be able to type any string into it.
It should save correctly.
Create another product
Fill in the name, and try to type the same value as you typed before for GTIN, UPC, EAN OR ISBN
Try to publish
An error should occur, as we don't allow duplicated values.

Changelog entry

Automatically create a changelog entry from the details below.

Significance

Patch
Minor
Major

Type

Fix - Fixes an existing bug
Add - Adds functionality
Update - Update existing functionality
Dev - Development related task
Tweak - A minor adjustment to the codebase
Performance - Address performance issues
Enhancement - Improvement to existing functionality

Message

Comment

github-actions · 2024-05-10T13:33:02Z

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

github-actions · 2024-06-20T18:52:57Z

Hi @louwie17,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

octaedro

Good job @nathanss! This is testing well here and the code LGTM.

However, I would like to ask a couple of questions. After answering them, I think this PR will be ready to go.

Could the unique_id be repeated? I mean, now it is possible to have them duplicated, but I was wondering if that was expected.
The SKU will continue being unique, right? Maybe we can add a comment explaining that briefly because the naming is not very intuitive.

nathanss · 2024-06-21T19:26:59Z

@octaedro these are all relevant questions, which I'll be redirecting to the product team @pmcpinto @j111q

Are we OK merging this in the state it is right now (which fulfills the acceptance criteria described in #39577), and adding additional features later? Or should we revisit this?

louwie17

Could the unique_id be repeated? I mean, now it is possible to have them duplicated, but I was wondering if that was expected.

This does seem like a critical piece we want to implement from the start, and something that is easier to implement now and than later on revert, versus the other way around.
I would say given this is a unique id, we should have it act similar as a SKU and not allow duplicates across products.
Did any of this come up in the discussions? or P2 conversations?

louwie17 · 2024-06-24T07:48:13Z

...ins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php

+					'type'        => 'string',
+					'context'     => array( 'view', 'edit' ),
+				),
+				'unique_id'             => array(


Hmm should the description here for unique_id be: 'GTIN, UPC, EAN or ISBN' ?
And we keep 'Unique identifier.' for the sku field?

I'm OK with renaming the unique_id field, but I think we should avoid saying that sku is an 'Unique identifier', since we're introducing a new field with this name.

Just for clarification: sku is unique at the store level vs. the new field shall hold a globally unique identifier such as GTIN.
Hence, the proposed new unique_id would also be unique at the store level.
Maybe calling it global_unique_id could be more explicit?

I like it. I will rename it

pmcpinto · 2024-06-24T10:15:41Z

This does seem like a critical piece we want to implement from the start, and something that is easier to implement now and than later on revert, versus the other way around.
I would say given this is a unique id, we should have it act similar as a SKU and not allow duplicates across products.

I agree that we should avoid duplications.

github-actions · 2024-06-24T19:53:14Z

Size Change: +37 B (0%)

Total Size: 2.49 MB

_{compressed-size-action}

louwie17

Nice work @nathanss, it tested well!
I left a couple small comments, but for the rest this looks great!

louwie17 · 2024-07-01T13:50:53Z

plugins/woocommerce/includes/abstracts/abstract-wc-product.php

+	/**
+	 * Get Unique ID.
+	 *
+	 * @since 9.0.0


Should be updated to 9.1 as well

louwie17 · 2024-07-01T14:06:03Z

...merce/includes/rest-api/Controllers/Version3/class-wc-rest-product-variations-controller.php

@@ -535,7 +541,12 @@ public function get_item_schema() {
 					'readonly'    => true,
 				),
 				'sku'                   => array(
-					'description' => __( 'Unique identifier.', 'woocommerce' ),
+					'description' => __( 'SKU.', 'woocommerce' ),


Should we still keep this as 'Unique identifier.'?

I prefer keeping it like this, to avoid confusion with the new field, though I don't think this description is a big deal. I'll update it to Stock Keeping Unit, is that OK?

louwie17 · 2024-07-01T14:06:20Z

...ins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php

@@ -987,7 +992,12 @@ public function get_item_schema() {
 					'context'     => array( 'view', 'edit' ),
 				),
 				'sku'                   => array(
-					'description' => __( 'Unique identifier.', 'woocommerce' ),
+					'description' => __( 'SKU.', 'woocommerce' ),


Same question as above.

MatthiasReinholz

Thank you @nathanss , I tested this and it looks good to me!

I have two questions:

The new field is only available on the new product editor, correct?
Will the global_unique_id be populated to the REST API?

nathanss · 2024-07-02T19:47:20Z

@MatthiasReinholz

The new field is only available on the new product editor, correct?

Yes, currently, that is correct.

Will the global_unique_id be populated to the REST API?

It is added to the REST API. The new product editor consumes the REST API, so it will be available there immediately.

louwie17

Thanks for the API description updates using Stock Keeping Unit. seems good with me.
I tested this again and it seems to test well, nice work, I also noticed that the wcadmin_product_add_publish track includes the global_unique_id id 👍

* Add unique_id field to product * Fix documentation * Fix wrong comment * Update description * Add unique_id for variations * Update lint * Update tests * Add changelog * Update variations unit test * Add unique_id to wc_product_meta_lookup * Add unique_id methods to cpt interface * Add new methods for unique_id and add it to lookup table * Add unique_id functions to product functions * Throw error when unique_id is duplicated * Handle error message for unique_id on the front-end * Add changelog * Rename unique_id description in REST API * Fix lint issues * Add unique_id tracking for product publish * Rename to 'global_unique_id' * Fix lint * Fix lint * Bump documentation version * Update controller description of fields

nathanss marked this pull request as draft May 10, 2024 13:25

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 10, 2024

nathanss force-pushed the poc/unique_id_field branch from a36a2b4 to 3d28f75 Compare June 20, 2024 17:49

nathanss self-assigned this Jun 20, 2024

nathanss marked this pull request as ready for review June 20, 2024 17:51

nathanss requested review from a team and octaedro June 20, 2024 18:52

octaedro reviewed Jun 21, 2024

View reviewed changes

louwie17 reviewed Jun 24, 2024

View reviewed changes

nathanss closed this Jun 28, 2024

nathanss reopened this Jun 28, 2024

nathanss closed this Jun 29, 2024

nathanss reopened this Jun 29, 2024

nathanss force-pushed the poc/unique_id_field branch from c9f1144 to ca02069 Compare July 1, 2024 13:46

nathanss requested a review from MatthiasReinholz July 1, 2024 13:49

louwie17 reviewed Jul 1, 2024

View reviewed changes

nathanss added 9 commits July 1, 2024 16:53

Add unique_id field to product

b40358c

Fix documentation

db13301

Fix wrong comment

d4878d0

Update description

2bf505d

Add unique_id for variations

264d395

Update lint

063b53b

Update tests

28fdbfe

Add changelog

cc9f8ba

Update variations unit test

229e49d

nathanss added 14 commits July 1, 2024 16:53

Add unique_id methods to cpt interface

80c372c

Add new methods for unique_id and add it to lookup table

d6f9ef4

Add unique_id functions to product functions

fd85ff7

Throw error when unique_id is duplicated

48600a1

Handle error message for unique_id on the front-end

432e386

Add changelog

63bcf39

Rename unique_id description in REST API

2dfb56d

Fix lint issues

5daa461

Add unique_id tracking for product publish

196ef16

Rename to 'global_unique_id'

65d8605

Fix lint

2640759

Fix lint

f562a7d

Bump documentation version

4fecd24

Update controller description of fields

f4cc4a9

nathanss force-pushed the poc/unique_id_field branch from 8544974 to f4cc4a9 Compare July 1, 2024 19:53

MatthiasReinholz approved these changes Jul 2, 2024

View reviewed changes

nathanss requested a review from louwie17 July 2, 2024 22:28

louwie17 approved these changes Jul 3, 2024

View reviewed changes

nathanss merged commit 85cedf2 into trunk Jul 3, 2024
40 checks passed

nathanss deleted the poc/unique_id_field branch July 3, 2024 14:45

github-actions bot added this to the 9.2.0 milestone Jul 3, 2024

github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Jul 3, 2024

peterfabian added the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label Jul 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unique_id field to product #47364

Add unique_id field to product #47364

nathanss commented May 10, 2024 •

edited

Loading

github-actions bot commented May 10, 2024 •

edited

Loading

github-actions bot commented Jun 20, 2024 •

edited

Loading

octaedro left a comment

nathanss commented Jun 21, 2024

louwie17 left a comment

louwie17 Jun 24, 2024

nathanss Jun 24, 2024

MatthiasReinholz Jun 27, 2024

nathanss Jun 28, 2024

pmcpinto commented Jun 24, 2024

github-actions bot commented Jun 24, 2024 •

edited

Loading

louwie17 left a comment

louwie17 Jul 1, 2024

louwie17 Jul 1, 2024

nathanss Jul 1, 2024

louwie17 Jul 1, 2024

MatthiasReinholz left a comment

nathanss commented Jul 2, 2024 •

edited

Loading

louwie17 left a comment

Add unique_id field to product #47364

Add unique_id field to product #47364

Conversation

nathanss commented May 10, 2024 • edited Loading

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Changelog entry

Significance

Type

Message

Comment

github-actions bot commented May 10, 2024 • edited Loading

Test using WordPress Playground

github-actions bot commented Jun 20, 2024 • edited Loading

octaedro left a comment

Choose a reason for hiding this comment

nathanss commented Jun 21, 2024

louwie17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmcpinto commented Jun 24, 2024

github-actions bot commented Jun 24, 2024 • edited Loading

louwie17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MatthiasReinholz left a comment

Choose a reason for hiding this comment

nathanss commented Jul 2, 2024 • edited Loading

louwie17 left a comment

Choose a reason for hiding this comment

nathanss commented May 10, 2024 •

edited

Loading

github-actions bot commented May 10, 2024 •

edited

Loading

github-actions bot commented Jun 20, 2024 •

edited

Loading

github-actions bot commented Jun 24, 2024 •

edited

Loading

nathanss commented Jul 2, 2024 •

edited

Loading